Get rid of github.com/go-chi/render use#1919
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request removes the dependency on github.com/go-chi/render by replacing its usage with direct response writing and a custom WriteJSON helper, thereby standardizing the response handling across backend REST endpoints.
- Replaced all render.HTML, render.JSON, and render.Status calls with w.WriteHeader, w.Write, and rest.WriteJSON.
- Updated JSON decoding in tests and controllers to use encoding/json instead of render.DecodeJSON.
- Removed the go-chi/render import from multiple files.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backend/app/rest/httperrors.go | Removed render calls; added the WriteJSON helper function. |
| backend/app/rest/api/rest_public.go | Replaced render methods with rest.WriteJSON and json.Decoder. |
| backend/app/rest/api/rest_private_test.go | Updated tests to use json.Unmarshal instead of render.DecodeJSON. |
| backend/app/rest/api/rest_private.go | Replaced render calls with rest.WriteJSON in various controllers. |
| backend/app/rest/api/rest.go | Replaced render calls with rest.WriteJSON. |
| backend/app/rest/api/migrator.go | Replaced render calls with rest.WriteJSON for import endpoints. |
| backend/app/rest/api/admin.go | Replaced render calls with rest.WriteJSON in admin handlers. |
Files not reviewed (1)
- backend/go.mod: Language not supported
Comments suppressed due to low confidence (1)
backend/app/rest/api/admin.go:200
- [nitpick] The JSON key 'read-only' uses a hyphen; consider using camelCase (e.g., 'readOnly') for better consistency and to avoid potential client parsing issues.
rest.WriteJSON(w, http.StatusOK, R.JSON{"locator": locator, "read-only": roStatus})
8605920 to
93cdd50
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR removes the dependency on github.com/go-chi/render and replaces its usage with custom response writers. Key changes include:
- Removing go-chi/render from imports and replacing its methods with rest.WriteJSON and explicit response writes.
- Modifying controllers and tests to use standard json encoding/decoding.
- Updating error and HTML response handling to ensure proper headers and status codes.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backend/app/rest/httperrors.go | Replaces go-chi/render with custom WriteJSON and explicit HTML response handling |
| backend/app/rest/api/rest_public.go | Removes go-chi/render usage; updates JSON and plain text response logic |
| backend/app/rest/api/rest_private_test.go | Adjusts tests to use json.Unmarshal instead of render.DecodeJSON |
| backend/app/rest/api/rest_private.go | Updates multiple endpoints to use WriteJSON and explicit header/status writing |
| backend/app/rest/api/rest.go | Removes go-chi/render, using WriteJSON for consistent JSON responses |
| backend/app/rest/api/migrator.go | Replaces go-chi/render with WriteJSON in import and wait endpoints |
| backend/app/rest/api/admin.go | Updates admin endpoints to use WriteJSON instead of go-chi/render |
Files not reviewed (1)
- backend/go.mod: Language not supported
Comments suppressed due to low confidence (1)
backend/app/rest/api/admin.go:200
- [nitpick] The JSON key 'read-only' may be unclear or inconsistent with common naming conventions; consider renaming it to 'readonly' for consistency and clarity.
rest.WriteJSON(w, http.StatusOK, R.JSON{"locator": locator, "read-only": roStatus})
93cdd50 to
a8d5964
Compare
a8d5964 to
d2ddedd
Compare
Replace go-chi/render with go-pkgz/rest for JSON responses and custom helpers for HTML/plain text responses. Key changes: - Replace render.JSON/render.Status with rest.RenderJSON and explicit w.WriteHeader() calls - Replace render.DecodeJSON with json.NewDecoder().Decode() - Add SendErrorJSON helper that sets Content-Type header before WriteHeader (required since rest.RenderJSON can't set headers after WriteHeader is called) - Add HTMLResponse and PlainTextResponse helpers Fix export double-execution in migrator.go: The original code called Export twice - once to io.Discard to check for errors, then again to actually write. This was wasteful and had a race condition risk. Now file mode buffers to memory first for atomic success/failure, while stream mode writes directly with proper error handling.
d2ddedd to
dedac80
Compare
Pull Request Test Coverage Report for Build 19895196749Details
💛 - Coveralls |
umputun
left a comment
There was a problem hiding this comment.
LGTM - clean removal of go-chi/render dependency with proper replacement using go-pkgz/rest
Replace go-chi/render with go-pkgz/rest for JSON responses and custom
helpers for HTML/plain text responses.
Key changes:
w.WriteHeader() calls
WriteHeader (required since rest.RenderJSON can't set headers after
WriteHeader is called)
Fix export double-execution in migrator.go:
The original code called Export twice - once to io.Discard to check for
errors, then again to actually write. This was wasteful and had a race
condition risk. Now file mode buffers to memory first for atomic
success/failure, while stream mode writes directly with proper error
handling.